Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create notifications #37

Closed
wants to merge 14 commits into from
Closed

Conversation

fluix-dev
Copy link
Collaborator

@fluix-dev fluix-dev commented Sep 17, 2020

The UI for notifications is still necessary.

Fixes #12
Fixes #13

@fluix-dev fluix-dev changed the title Create notification model Create notifications Sep 17, 2020
@fluix-dev
Copy link
Collaborator Author

@skyflaren please do the frontend for this.

@fluix-dev fluix-dev force-pushed the notification branch 2 times, most recently from 87bc829 to 13290a8 Compare September 19, 2020 23:17
@fluix-dev fluix-dev marked this pull request as draft September 20, 2020 00:06
app/templates/notifications.html Outdated Show resolved Hide resolved
app/templates/notifications.html Outdated Show resolved Hide resolved
{% if notif != notifications[0] and notif.notification.linked_item.__str__() != notifications[0].notification.linked_item.__str__() %}
<hr>
{% endif %}
<a class="notification-link font-alt font-bold" href="{{ notif.get_link_url() }}">{{ notif.notification.linked_item.__str__() }}</a>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need the __str__()?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wait yeah you're right

app/templates/notifications.html Outdated Show resolved Hide resolved
app/templates/userhome.html Outdated Show resolved Hide resolved
app/views.py Outdated Show resolved Hide resolved
@fluix-dev fluix-dev requested a review from dulldesk October 1, 2020 03:04
{% endif %}
<a class="notification-link font-alt font-bold" href="{{ notif.get_link_url() }}">{{ notif.notification.linked_item.__str__() }}</a>
<span class="font-light">{{ notif.message }}</span><br>
{% endfor %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not high priority but could add a message for when there are no notifications.

@dulldesk
Copy link
Contributor

dulldesk commented Oct 1, 2020

Notifications do not seem to be sorted (and it seems as if you cannot sort by property [as opposed to field]) ¯_(ツ)_/¯

@fluix-dev
Copy link
Collaborator Author

Notifications do not seem to be sorted (and it seems as if you cannot sort by property [as opposed to field]) ¯_(ツ)_/¯

You can just sort by notification__created instead of the property.

@fluix-dev fluix-dev marked this pull request as ready for review October 1, 2020 03:47
@@ -122,6 +122,13 @@ footer {
font-size: 1.5rem;
}

hr{
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
hr{
hr {

@@ -212,6 +219,14 @@ footer {
font-weight: 300;
}

.notification-link{
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.notification-link{
.notification-link {

color: var(--cirrus-primary);
}

.notification-link:hover{
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.notification-link:hover{
.notification-link:hover {

@fluix-dev fluix-dev closed this Mar 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notifications Following
3 participants